-
Notifications
You must be signed in to change notification settings - Fork 0
[Feat] 기본적인 Auth 기능 구현 #6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughSpring Security 및 JJWT 의존성 추가, JPA Auditing 활성화. 공통 응답/상태·전역 예외 처리 추가. JWT 제공자·필터·UserDetailsService·보안 설정 및 인증 API/서비스/DTO/리포지토리 추가. 다수 엔티티의 PK 타입 String→Long 변경 및 Users에 password 칼럼 추가. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant AuthController as AuthController
participant AuthService as AuthService
participant UserRepository as UserRepository
participant JwtTokenProvider as JwtTokenProvider
rect rgba(220,100,100,0.5)
Note over Client,AuthController: 로그인 흐름
Client->>AuthController: POST /api/auth/login (studentNum,password)
AuthController->>AuthService: login(request)
AuthService->>UserRepository: findByStudentNum(studentNum)
UserRepository-->>AuthService: Users
AuthService->>AuthService: 비밀번호 검증 및 상태 체크
AuthService->>JwtTokenProvider: createAccessToken(studentNum)
JwtTokenProvider-->>AuthService: accessToken
AuthService->>JwtTokenProvider: createRefreshToken(studentNum)
JwtTokenProvider-->>AuthService: refreshToken
AuthService-->>AuthController: TokenResponse
AuthController-->>Client: BaseResponse<TokenResponse>
end
sequenceDiagram
actor Client
participant Filter as JwtAuthenticationFilter
participant JwtProvider as JwtTokenProvider
participant UserDetailsService as CustomUserDetailsService
participant SecurityContext as SecurityContext
rect rgba(100,220,100,0.5)
Note over Client,Filter: 요청 → JWT 검증 흐름
Client->>Filter: 요청 (Authorization: Bearer {token})
Filter->>JwtProvider: validateToken(token)
JwtProvider-->>Filter: valid / throws
Filter->>JwtProvider: getAuthentication(token)
JwtProvider->>UserDetailsService: loadUserByUsername(studentNum)
UserDetailsService-->>JwtProvider: UserDetails
JwtProvider-->>Filter: Authentication
Filter->>SecurityContext: setAuthentication(auth)
Filter-->>Client: 다음 필터/컨트롤러로 전달
end
sequenceDiagram
actor Client
participant AuthController as AuthController
participant AuthService as AuthService
participant JwtTokenProvider as JwtTokenProvider
participant UserRepository as UserRepository
rect rgba(100,100,220,0.5)
Note over Client,AuthController: 리프레시 토큰 재발급 흐름
Client->>AuthController: POST /api/auth/refresh (Authorization: Bearer {refreshToken})
AuthController->>AuthService: refresh(refreshToken)
AuthService->>JwtTokenProvider: validateToken(refreshToken)
JwtTokenProvider-->>AuthService: valid
AuthService->>JwtTokenProvider: isRefreshToken(refreshToken)
JwtTokenProvider-->>AuthService: true/false
AuthService->>JwtTokenProvider: getStudentNum(refreshToken)
JwtTokenProvider-->>AuthService: studentNum
AuthService->>UserRepository: findByStudentNum(studentNum)
UserRepository-->>AuthService: Users
AuthService->>JwtTokenProvider: createAccessToken / createRefreshToken
JwtTokenProvider-->>AuthService: new tokens
AuthService-->>AuthController: TokenResponse
AuthController-->>Client: BaseResponse<TokenResponse>
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/ssurent/ssurentbe/domain/users/entity/Users.java (1)
18-33: 스키마 마이그레이션/백필 없이는 운영 장애 가능성이 큽니다.
PK 타입 변경과password NOT NULL추가는 기존 데이터가 있는 환경에서 마이그레이션 실패나 런타임 오류를 유발할 수 있습니다. Flyway/Liquibase로 PK 타입 변경, 기존 사용자 비밀번호 백필(또는 단계적 nullable 전환) 계획을 명확히 해 주세요.
🤖 Fix all issues with AI agents
In `@build.gradle`:
- Around line 49-55: Update the springdoc-openapi dependencies to the 3.0.x line
and add a Bouncy Castle override: replace the existing springdoc coordinates
(org.springdoc:springdoc-openapi-starter-webmvc-ui and
org.springdoc:springdoc-openapi-starter-webmvc-api currently at 2.8.13) with
3.0.0 (or latest 3.0.x) to be compatible with Spring Boot 4.0.x, and add an
explicit org.bouncycastle:bcpkix-jdk18on dependency at 1.79+ to override the
transitive Bouncy Castle pulled in by io.jsonwebtoken:jjwt-*
(jjwt-api/jjwt-impl/jjwt-jackson) and mitigate CVE-2025-8916.
In
`@src/main/java/ssurent/ssurentbe/common/exception/GlobalExceptionHandler.java`:
- Around line 12-17: The handler in
GlobalExceptionHandler.handleGeneralException currently casts e.getStatus() to
ErrorStatus which can throw ClassCastException at runtime; change the
implementation to avoid unsafe casting by checking e.getStatus() with instanceof
(or using a safe accessor) and handle non-ErrorStatus cases (e.g., map unknown
statuses to a default ErrorStatus or return a generic error response) before
calling BaseResponse.error; ensure you reference GeneralException.getStatus()
and ErrorStatus and produce a ResponseEntity with an appropriate HttpStatus for
fallback paths.
In `@src/main/java/ssurent/ssurentbe/common/jwt/JwtAuthenticationFilter.java`:
- Around line 42-44: JwtAuthenticationFilter currently unsafely casts
GeneralException.getStatus() to ErrorStatus in the catch block; update the
handling so the cast is safe by either (A) changing GeneralException to only
accept ErrorStatus in its constructor (and update usages) or (B) adding a
runtime check in JwtAuthenticationFilter (and similarly in
GlobalExceptionHandler) that tests if e.getStatus() instanceof ErrorStatus
before casting—if true, call sendErrorResponse(response, (ErrorStatus)
e.getStatus()); otherwise map/convert the BaseStatus to a safe ErrorStatus (or
use a default/generic error response) and log the unexpected status type. Ensure
you update both JwtAuthenticationFilter and GlobalExceptionHandler to follow the
same safe pattern.
In
`@src/main/java/ssurent/ssurentbe/common/security/CustomUserDetailsService.java`:
- Around line 22-31: loadUserByUsername currently only checks existence and
returns a UserDetails, allowing deleted/disabled accounts to authenticate;
update CustomUserDetailsService.loadUserByUsername to check the Users entity
flags (e.g., isDeleted() and getStatus()/status enum) after fetching from
userRepository and throw UsernameNotFoundException or a custom DisabledException
when the account is deleted or inactive. Ensure the exception uses a clear
ErrorStatus (or map to ErrorStatus.USER_NOT_FOUND or a new ErrorStatus for
disabled users) so getAuthentication() will reject such tokens, and keep
returning the Spring Security User with roles only for active users.
In `@src/main/java/ssurent/ssurentbe/domain/users/controller/AuthController.java`:
- Around line 41-43: AuthController.refresh currently unsafely extracts the
token with authorization.replace("Bearer ", "") which can NPE or return wrong
value; change it to the same safe parsing used by
JwtAuthenticationFilter.resolveToken — i.e., check for null/empty, verify the
header startsWith("Bearer "), then extract the substring after the prefix (or
call JwtAuthenticationFilter.resolveToken(authorization) if accessible) and
handle a missing/invalid token by returning an appropriate error (or throwing
the same exception path used elsewhere) before calling
authService.refresh(refreshToken).
In
`@src/main/java/ssurent/ssurentbe/domain/users/controller/docs/AuthApiDocs.java`:
- Around line 21-46: Create concrete wrapper response classes that extend
BaseResponse with the generic type parameter (e.g., class SignupResponse extends
BaseResponse<TokenResponse>, LoginResponse extends BaseResponse<TokenResponse>,
RefreshResponse extends BaseResponse<TokenResponse>) and update the
`@ApiResponse/`@Content annotations on the signup, login and refresh operations to
use `@Schema`(implementation = SignupResponse.class) / LoginResponse.class /
RefreshResponse.class respectively so springdoc-openapi can surface
TokenResponse fields (accessToken, refreshToken, tokenType) in Swagger; locate
the annotations around the methods named signup, login and the token refresh
operation and replace the existing `@Schema`(implementation = BaseResponse.class)
references with the new concrete classes.
In `@src/main/java/ssurent/ssurentbe/domain/users/dto/SignupRequest.java`:
- Around line 3-8: SignupRequest currently uses the record-generated toString
which exposes sensitive fields; override SignupRequest#toString to return a safe
representation that masks phoneNum and password (e.g., show only last 2-4
digits/characters or replace with fixed ********), keep non-sensitive fields
like name/studentNum readable if desired, and ensure the method is implemented
on the record itself so any logging/exception handling uses the masked output.
In `@src/main/java/ssurent/ssurentbe/domain/users/service/AuthService.java`:
- Around line 69-72: The if-check in AuthService.refresh around
jwtTokenProvider.validateToken(refreshToken) is dead because validateToken
throws GeneralException on invalid tokens; remove the if-block and either (a)
simply call jwtTokenProvider.validateToken(refreshToken) and let its exception
propagate, or (b) wrap the call in a try-catch that catches the thrown exception
and rethrows/translate it to ErrorStatus.JWT_INVALID for clearer control flow;
update the method to proceed to the rest of TokenResponse creation only after a
successful validateToken call.
🧹 Nitpick comments (5)
src/main/java/ssurent/ssurentbe/common/status/ErrorStatus.java (1)
12-18: 사용하지 않는 중복 상수 제거
COMM_ERROR_STATUS는BAD_REQUEST와 동일한 코드/메시지를 가지고 있으며 코드베이스에서 사용되지 않습니다. 중복 제거를 위해 삭제하는 것을 권장합니다.src/main/java/ssurent/ssurentbe/domain/users/dto/LoginRequest.java (1)
3-6: 선택적: record의 민감정보 보호를 위한 toString() 오버라이드LoginRequest는 password를 포함하는 record입니다. 현재 코드에서 요청 객체가 명시적으로 로깅되지는 않으나, Spring 내부 로깅, 모니터링 도구, 또는 예외 스택트레이스에서 기본 toString()이 호출될 가능성이 있습니다. 민감정보 보호를 위해 password를 마스킹하는 것을 권장합니다.
제안 수정
public record LoginRequest( String studentNum, String password ) { + `@Override` + public String toString() { + return "LoginRequest[studentNum=" + studentNum + ", password=***]"; + } }src/main/java/ssurent/ssurentbe/common/config/SecurityConfig.java (1)
25-37: CORS 설정이 누락되어 있습니다.프론트엔드 애플리케이션에서 API를 호출할 경우, CORS(Cross-Origin Resource Sharing) 설정이 없으면 브라우저에서 요청이 차단될 수 있습니다. 프론트엔드와 백엔드가 다른 도메인/포트에서 실행된다면 CORS 설정을 추가하는 것을 고려하세요.
♻️ CORS 설정 추가 예시
http + .cors(cors -> cors.configurationSource(corsConfigurationSource())) .csrf(csrf -> csrf.disable()) .sessionManagement(session -> session.sessionCreationPolicy(SessionCreationPolicy.STATELESS)) // ... 나머지 설정별도의
CorsConfigurationSource빈을 정의하거나,@CrossOrigin어노테이션을 컨트롤러에 추가할 수 있습니다.src/main/java/ssurent/ssurentbe/domain/users/service/AuthService.java (1)
51-67: 로그인 시 비밀번호 검증 후 isDeleted 체크 순서를 고려하세요.현재 구현은 비밀번호가 맞더라도 탈퇴한 사용자에게
USER_WITHDRAWN에러를 반환합니다. 이는 공격자에게 해당 학번으로 탈퇴한 계정이 존재한다는 정보를 노출할 수 있습니다. 보안 강화를 위해 탈퇴 여부와 관계없이 동일한INVALID_CREDENTIALS에러를 반환하는 것을 고려해 볼 수 있습니다.src/main/java/ssurent/ssurentbe/common/jwt/JwtTokenProvider.java (1)
75-93:validateToken메서드의 반환 타입이 실제 동작과 불일치합니다.이 메서드는
boolean을 반환하도록 선언되어 있지만, 실제로는 유효한 토큰일 때만true를 반환하고, 유효하지 않은 토큰일 때는false대신 예외를 던집니다. 이로 인해 호출하는 코드에서if (!validateToken(token))과 같은 불필요한 조건문이 작성될 수 있습니다 (AuthService.refresh참조).반환 타입을
void로 변경하거나, 메서드 이름을validateTokenOrThrow로 변경하여 예외를 던진다는 것을 명확히 하는 것을 권장합니다.♻️ 수정 제안
-public boolean validateToken(String token) { +public void validateToken(String token) { try { Jwts.parser() .verifyWith(secretKey) .build() .parseSignedClaims(token); - return true; } catch (SignatureException e) { throw new GeneralException(ErrorStatus.JWT_INVALID_SIGNATURE); } // ... 나머지 catch 블록 }
src/main/java/ssurent/ssurentbe/common/exception/GlobalExceptionHandler.java
Show resolved
Hide resolved
src/main/java/ssurent/ssurentbe/common/security/CustomUserDetailsService.java
Show resolved
Hide resolved
src/main/java/ssurent/ssurentbe/domain/users/controller/AuthController.java
Show resolved
Hide resolved
src/main/java/ssurent/ssurentbe/domain/users/controller/docs/AuthApiDocs.java
Show resolved
Hide resolved
src/main/java/ssurent/ssurentbe/domain/users/service/AuthService.java
Outdated
Show resolved
Hide resolved
fprtmjinho
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아직 뭐가 뭔지 몰라서 보기만했어요
- 기존 BaseStatus로 받을 시 SuccessStatus 가 들어올 시 런타임 에러 발생 가능성 존재. - ErrorStatus만 받도록 수정
- 도달하지 않는 조건 제거
- isDeleted는 권장되지 않기 때문에 deleted로 변경
Penalty Entity 삭제 PenaltyTypes 변경
Penalty Entity 삭제 PenaltyTypes 변경
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/main/java/ssurent/ssurentbe/domain/users/enums/PanaltyTypes.java`:
- Around line 3-5: Add `@Enumerated`(EnumType.STRING) to the
UserPanaltyLog.panaltyType field so PanaltyTypes is persisted by name instead of
ordinal; update the PanaltyTypes enum definition (public enum PanaltyTypes) only
if needed to include any legacy constants (e.g., BANNED) to avoid
compile/runtime mismatches. Also add a DB migration that converts existing
ordinal values to their String names (e.g., rows where panalty_type = 2 ->
'BANNED', 1 -> 'UNAUTHORIZED_USE', 0 -> 'OVERDUE') and run it before deploying
the annotation change to prevent data corruption.
🧹 Nitpick comments (1)
src/main/java/ssurent/ssurentbe/common/config/SwaggerConfig.java (1)
18-31: 전역 SecurityRequirement로 인해 공개 엔드포인트도 인증 필요로 표시됩니다.회원가입/로그인처럼 공개되어야 하는 API까지 Swagger에서 인증 필요로 보일 수 있습니다. 전역 적용을 제거하고, 보안이 필요한 컨트롤러/메서드에만
@SecurityRequirement를 붙이는 방식이 더 정확합니다.♻️ 제안 변경 (전역 적용 제거)
return new OpenAPI() .info(new Info() .title("SSURent API") .description("SSURent 백엔드 API 문서") .version("v1.0.0")) - .addSecurityItem(new SecurityRequirement().addList(securitySchemeName)) .components(new Components() .addSecuritySchemes(securitySchemeName, new SecurityScheme()
개요
PR 유형
어떤 변경 사항이 있나요?
PR Checklist
PR이 다음 요구 사항을 충족하는지 확인하세요.
🧩 작업 내용
📸 스크린샷(선택)
📣 To Reviewers
Summary by CodeRabbit
New Features
Behavior / UX
Security
Data Model
Docs